@ellisnet,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla2.dotnetfoundation.org.

It will cover your contributions to all .NET Foundation-managed open source projects.
Thanks,
.NET Foundation Pull Request Bot

@ellisnet, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, .NET Foundation Pull Request Bot

I think nobody is interested in Modules that do not register types. Thus on-demand (that is, late) loading of such a module makes no sense.

A better, straightforward, approach IMO would be:

Declare up front that on-demand loading is not supported.
If any of modules define InitializationMode=OnDemand, throw NotImplementedException.
Then load all modules during initialization.

@dvorn I disagree completely. Registration of types in the IoC container is just one thing that you might want to do in your module initialization routine. For example, if your module had something to do with interacting with a SQLite database, you might need to check to see if the database exists or not, and create it if not. But this might never need to happen, based on whether the user uses that function of your app or not. So it makes perfect sense that you would register any types in the IModule.RegisterTypes() method; but do other initialization in the IModule.Initialize() method (e.g. create the database as needed) - and this initialization could still happen OnDemand.
Also, I described a possibility in the discussion for issue #969 - that you might have a set of modules that do various things; and you might want to take all of the type registration code and put it in a single (dedicated?) module, so that all of your other modules could still have normal OnDemand or WhenAvailable initialization according to your wishes.
I feel pretty strongly about this: You really do not want to be forced to do all of your module initialization during app startup if you can help it; by designing things carefully, you can have that stuff happen when needed or during a time when the user expects the app to be "busy" - making you do it all during app initialization will hurt your ability to minimize your app's startup time.

It would probably have been easier to add a RegisterTypes() method to the IModule definition - that matches how the rest of Prism.Forms works. Have a type to register? Put it in the RegisterTypes() method...
But I didn't think that the scope of my project allowed for making changes to the definition of IModule, since that is defined outside of the Prism.Autofac.Forms library.

@ellisnet Please take a look http://prismlibrary.readthedocs.io/en/latest/WPF/04-Modules/ It's for WPF, but ideas are the same.

During module initialization, the module can retrieve references to the additional components and services it requires, and/or register any components and services that it contains in order to make them available to other modules.

@ellisnet

to add a RegisterTypes() method

You do not need this method. This method already exists and it is called Initialize. This is the design of Prism.

@dvorn Thank you. The link that you sent was helpful; I did read through it and it is clear why you would not want to modify the definition of IModule specifically for Prism.Autofac.Forms.
Obviously there are differences between how Prism for WPF works and how Prism.Forms needs to work - mobile devices have different needs than computers, AOT compilation for iOS presents new problems (e.g. I think that loading extra app functionality on the fly via modules is restrictive), etc.

Also, it seems like the Autofac implementation for Prism.Forms could have some minor differences from the Unity implementation. So, I would ask that - at least with respect to this "feature" of Prism.Autofac.Forms - the pull request be accepted as-is. As it is programmed, it allows for increased flexibility for module and app developers (vs. requiring them to only do one type of module initialization).

Finally, I believe that your suggestion could introduce a new problem: If the Initialize() method for a module was called before the IoC container is built (as is necessary when doing type registration), then no type resolution from the container can occur in the Initialize() method. Module developers could only do type registration or type resolution in the Initialize() method - not both. This will be confusing for new Prism (with Autofac) developers - where telling them they have to do their type registration in a RegisterTypes() method is more straightforward (since that is where they have to do it in other places in Prism.Forms).

Thank you for your time and thoughts.

@dvorn I wanted to add one more comment:
At one point, I thought Brian L. wanted someone to update Prism with Autofac for (a) Xamarin.Forms, (b) UWP and (c) WPF all at the same time. It is possible that I just read one of his comments incorrectly... However, last week I connected with him, and we agreed that I should only worry about Prism.Autofac.Forms for now. I do not plan to work on Prism with Autofac for UWP (without Xamarin.Forms) or WPF at this time.

So I am not sure if you had a concern that the changes that I made in Prism.Autofac.Forms would be replicated in those other platforms; but at this time they won't be.

Sorry for any confusion about that.

@ellisnet I was wrong stating that you don't need RegisterTypes method, sorry for that. You are absolutely right, immutable containers need two entry points - one for registering services in this module and another one for configuring services in another modules.

Conceptually, something like

RegisterServices(ContainerBuilder builder)
{
    // register types in container
}
ConfigureServices(Container container)
{
    // use container to retrieve instances of other services and configure them
}

Note however that you still need to forbid on-demand modules. The module registers its types immediately on startup, but the services provided by the module may not be functional until the module performed initialization. Someone may use the services (registered types) before they were initialized properly.

@dvorn - I addressed the issue of using a ContainerBuilder vs. an AutofacContainer in a response on issue #969 - so no need to rehash that here.
In your comment above, you are proposing two methods for IModules - RegisterServices() and ConfigureServices(). I believe that the "standard" for Prism.Forms is for types to be registered in the container in a method called RegisterTypes(). So RegisterServices() -becomes-> RegisterTypes().
Then I don't know why you are proposing to replace Initialize() with ConfigureServices(). It seems like, when you have an immutable container where the module's types are registered separately from the module's initialization, the purpose of the Initialize() method is to "use container to retrieve instances of other services and configure them".

So, if you change your RegisterServices() method to RegisterTypes(); and your ConfigureServices() method to Initialize() (which is already defined in IModule); then what you are proposing is what I have submitted. :-)

I still do not agree with this statement: "you still need to forbid on-demand modules"
There is no reason at all that developers who consume Prism.Autofac.Forms shouldn't be able to decide whether they want their module's Initialize() method to run at app startup (WhenAvailable) or when the module is loaded by the app (OnDemand).

You wrote: "Someone may use the services (registered types) before they were initialized properly." I believe that the Prism framework itself handles the on-demand initialization via IModuleManager.LoadModule() - see example.
Developers who create/consume modules will just need to know they need to load their module via IModuleManager.LoadModule() before they work with types defined in that module - or mark their modules as WhenAvailable - that shouldn't be too difficult to understand and work with; and I believe that it is consistent with how Prism.Forms modules generally work (e.g. with Unity).

@ellisnet As for RegisterServices/ConfigureServices - I wrote "conceptually" and not as some API.

On-demand modules: you are just creating a trap for the users to fall into for no particular purpose. They will forget to call LoadModule and use types from the module which are readily available in the container from the very beginning. This will be a source of (sometimes very subtle) bugs.

@dvorn Understood.
I think that developers who create modules will likely be some of the more savvy Prism consumers. I think that they should be allowed to create modules which have delayed initialization - for the reasons specified above - with the understanding that they may need to guard [edit: was previously "should guard"] against registered types being used before the module is initialized/loaded (e.g. throw helpful exceptions). OR they can make it clear in the documentation of their module that the module is only supported with WhenAvailable initialization.

Autofac allows for complex registration/resolution logic, so something like this could be done - in RegisterTypes():

container.Register(ctx => isModuleInitialized ? new CustomService() : null)
   .As<ICustomService>();
//Or even better, throw a helpful exception

This is a much better situation than just saying that they can't create modules to be initialized OnDemand.

@ellisnet The net result is the same: the type is not usable until you load (initialize) the module, only the symptoms of the failure vary in difficulty to reveal what went wrong.

@dvorn I disagree. The type may not be usable until you initialize the module (in my example above, it was unusable). There may be registered types that are fine to be used prior to the module being initialized. It depends.
The best person to know (whether the registered type requires the module to be initialized or not) is the module developer. So my point was that we can let that person figure out how keep their modules from causing difficult-to-troubleshoot problems in apps that consume their modules.

@ellisnet when Autofac is updated to the latest v4.5 will this PR still be relevant?

Why, what is supposed to happen with the v4.5 release?

I completely tested it with Autofac 4.5.0, which has been available on NuGet since the beginning of April. The samples I tested it with are available here
If there is a change with Autofac 4.5.0 that might affect this PR, I am not aware of it. But I would certainly look into it, if you noticed something that made you wonder...

Just generally asking a question since I'm working to get Prism to support using the latest NetStandard releases

Apologies in advance if you know all of this already: I tested the PR quite thoroughly with projects that had been upgraded to current .NetStandard goodness. I spent quite a bit of time upgrading each of the various Prism for X.F samples to .NetStandard 1.4 per Oren Novotny's blog and then testing with latest Autofac. Those samples are the ones referenced in my previous comment.
It is my understanding that upgrading to .NetStandard 1.4 is best, since .NetStandard 2.0 is a superset of 1.4; and that updating to 1.5 or 1.6 could allow use of APIs that will not be included in 2.0. So I focused all of my efforts on 1.4.

This is the wrong thread to go into details on NetStandard versioning and should we compile Prism for multiple versions. That said if Prism supported only NetStandard1.4+ you would find it impossible to use Prism unless you first converted your PCL over to NetStandard1.4+

Yes. Thank you. I intended only to offer an explanation of why my testing was with 1.4. If someone suggested that this PR also needed to be tested against projects that had been upgraded 1.5 or 1.6, I would find time to do that.

Nope, I'm just not intimately familiar with Autofac, and wanted to be sure there weren't any breaking changes :)

I took a quick look at this and I do not feel comfortable having this abstraction around the container. I would prefer if we could just use the container directly.

@brianlagunas -
The PR - as it exists now, without your requested changes - ended up taking a lot more of my time than I expected. A significant amount of this time was spent trying to use Autofac in a way that did not require the container to be abstracted, but also made sense to a consumer of the library. This effort was not successful, so I created the AutofacContainer class (abstraction); and went with that. Also a lot of time was spent testing the version that is presented in this PR; testing that would need to be repeated with a re-formulated use of Autofac.
Obviously it makes no sense for me to complain about the amount of time that I voluntarily spent on an open source project - when others have dedicated far more time to their contributions. I just wanted to mention that a significant time went into the PR as it exists today. I will be looking at my availability for dedicating a similar amount of time to making the changes you have requested.
Thank you very much for your time, and for reviewing the PR code and sharing your thoughts.

@ellisnet from a consumer standpoint, you are most likely to choose a DI container you are already familiar with, and understand how to work with. Wrapping the container pushes the support for the container from the Autofac community to Prism.

Or standardize on IServiceProvider akin what Autofac.Extensions.DependencyInjection does, such that modules need not know about the implementation of the service provider. That limits them to use constructor injection only as that is the feature that every DI container supports.

I think conditional registration isn't quite often what you want. Rather, you register an interface that your module supports. Then you move your initialization from the Initialize method to a) the factory creating your implementation or b) the constructor of the implementation or c) you follow the factory or provider model, have consumers resolve the factory or provider instead of the feature itself and define that thing to do lazy or async initialization - whatever you want - and yield a properly initialized feature. If it is async and waiting e.g. for UI to be created, but also awaited during initialization, you have a deadlock. But you can test for that and throw a timeout with a proper error message.

IServiceProvider is not compatible with XF yet. I was looking into this already. I think when XF and Prism moves to .NET Standard this may be something we look into.

It seems this effort has stalled. I understand that this is a time consuming effort and I do not expect you to continue to meet our requested changes. I will close the PR and may refer to it when the time comes to implement this support.

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.